[Fix] Upload path_in_repo Normalization - #69
Merged
Merged
Conversation
…ssion tiers Audited the live OpenAPI document (1.1.0+master.20260813) against the client: of the 23 operations under the MCP and Studios tags, 8 were unimplemented and several implemented ones had drifted from the spec. Closes the gap and adds a guard so the next drift fails the suite instead of going unnoticed. Feature - Studios: list_studios, list_studio_hardware / _base_images / _sdk_versions, and the four plaintext-variable operations; exposed as `ms-hub studio list`, `studio variable`, `studio hardware|base-images|sdk-versions`, and `ms-hub list --repo-type studio`. - MCP: HubApi.list_operational_mcp_servers() plus `ms-hub mcp list --hosted`, which also unblocks the umbrella SDK delegating its own MCPApi here. - Studio visibility gains `protected` (app public, code hidden), previously inexpressible because visibility was folded into a `private` bool. - RepoInfo now carries the Studio runtime fields (sdk_type, sdk_version, base_image, hardware, mcp_support, runtime) instead of discarding them. - Token permission tiers: TokenScope annotations, a read-scoped token can log in (warning that writes need a higher tier), and a rejected write names the tier it requires. Fix - The compat layer forwarded `token` / `endpoint` into request bodies, so `HubApi().update_studio_settings(..., token=...)` serialised the caller's API token into the PATCH body. All Studio shims now split control kwargs from business fields. - `HubApi().get_studio_logs(..., token=...)` raised TypeError and could never succeed; a per-call `token=` was silently ignored on every Studio shim. - `cover_image` was renamed to `coverImage` on the way out and dropped by the server, so Studio cover images never applied. - 403 now separates insufficient permission from an exhausted quota (new QuotaExceededError, deliberately not retryable); 409 maps to AlreadyExistsError; the OpenAPI string error codes are recognised at all. - `get_studio` demanded a token for spaces the spec says are public. - `studio logs` read a `total` field the response never had, so its pagination footer never printed. Enhance - MCP discovery sends PUT first -- the only verb the spec defines -- and remembers which verb a deployment serves, so the loser is tried at most once. - Listing a Studio owner asks for every status: the endpoint keeps filtering to running spaces even with an owner set, so "list my spaces" returned nothing. Documentation - README: new Studio/MCP command surface, a Token permission levels section, and the SDK method reference. Testing - Vendored the spec under tests/data/ and check it against OPERATION_REGISTRY. - 723 unit tests (881 in mock mode) and 28 integration tests against the live service.
The News list had no entry for 0.3.0 or 0.3.1, so it jumped straight from 0.4.0 to 0.2.0 and left the release that took over all four console scripts undocumented. - Adds v0.3.0 (unified console scripts, repo/files truncation mitigation, get_current_username, agent visibility) and v0.3.1 (upload env var rename with deprecation shims, logout wiring, download lock paths, UserInfo and legacy compat field mapping), both reconstructed from the commits in each range. - Condenses v0.4.0 from eleven bullets to nine by merging entries that share a root cause: the three compat kwargs defects are one leak, and the two MCP/ Studio request-shape improvements are one behaviour change. - Folds v0.2.0 and everything older into the "Older releases" block, so only the three current releases are visible by default.
- v0.4.0: fold the second Feature bullet into the OpenAPI-coverage one, join the cover_image leak into the compat-kwarg fix, and merge the two small studio fixes into the error-handling bullet — nine bullets become six, one per category except Feature. - v0.3.0/v0.3.1: trim wordy phrasing and group Feature items first.
…eat/latest_openapi
…eat/latest_openapi
…eat/latest_openapi
…eat/latest_openapi
…eat/latest_openapi
…server limits Uploading a large dataset of small files was dominated by commit count and commit body size, not by bandwidth. Non-LFS files travel base64-inlined inside the commit request, so a 60k-file corpus put gigabytes through the API servers and forced tiny batches, multiplying commits against the per-repo hourly commit budget. Upload path: - LFS threshold becomes byte-level (MODELSCOPE_UPLOAD_LFS_FORCE_THRESHOLD, accepts 32KiB/512K/1MiB/0); the MB-only name stays as a deprecated alias. Default is unchanged at 1MiB. - UPLOAD_INLINE_METADATA_PATHS keeps README.md, .gitattributes and the configuration files inline whatever the threshold, so lowering it can never turn a file the Hub parses into an LFS pointer. - Commit batches are planned on two axes: operation count and inlined content volume (UPLOAD_COMMIT_MAX_INLINE_BYTES, 8MiB). Adaptive sizing no longer silently overrides an explicit cap, and both are clamped to the server's COMMIT_MAX_ACTIONS_PER_REQUEST ceiling, which is rejected rather than trimmed. - delete_files splits at the same ceiling instead of sending every path at once. - All LFS candidates are hashed up front and pre-signed in parallel groups, replacing one round trip per file. A group that fails degrades to per-file negotiation; an oid absent from a *failed* group is never read as "already exists", which would skip the transfer and commit a dangling pointer. Transport and resilience: - Both HTTP sessions size their pool from API_CONNECTION_POOL_MAXSIZE (32). urllib3's default of 10 discarded connections under the upload worker count and paid for a fresh TLS handshake on each. - Commit retries honor a server Retry-After, budgeted apart from the transient-error allowance, plus an opt-in per-hour commit governor shared by every commit path. upload_file now uses that path too. - A tree listing denied with 403 after the credential has been proven by an earlier successful read is retried; a large listing spans hundreds of requests, so a per-request denial rate otherwise guarantees whole-listing failure. Observability: - upload_folder takes progress_callback and emits per-batch commit events plus throttled wire-level upload_progress events carrying byte counts, so a consumer can build a rate and an ETA instead of watching a silent run. - upload_folder takes tracker_path, so resume state can live outside a staging tree that gets discarded between runs. - Invalid environment values now warn and name the fallback instead of reverting silently. Verified end to end against the Hub: 25000 files / 5.00 GiB and 10000 files / 2.00 GiB, both 0 failures, sha256 spot-checks identical, commit count 254 -> 49 and inlined commit volume 1.96GiB -> 55MiB on the 5 GiB corpus.
…overed work Batch pre-signing asks the server about every distinct oid before any upload starts, so the server cannot answer "already stored" for a duplicate -- it is not stored yet. Confirmed against the Hub: pre-signing the same fresh oid twice returns two upload URLs, and only after the blob lands does the endpoint report it as existing. Every occurrence of identical content therefore PUT the same bytes, which the previous per-file pre-sign path had avoided by accident of timing. Owner election, decided before any worker starts: - The first file holding an oid owns its transfer; the rest carry a marker and skip the PUT. `files_to_upload` is in ascending file index and a batch owns a contiguous ascending range, so an owner always lands in a batch no later than its copies. Batches commit in order and each waits for its own files, so a copy's blob is guaranteed present by the time its batch commits -- no locks, no waiting, and no way for the worker pool to block on itself. - Content the server already holds needs no owner at all. - If an owner's upload fails, its copies are withheld from the commit and pushed to the retry queue rather than published as pointers to a blob that was never stored. - Inline files are untouched: their content travels in the commit body, so there is no transfer to skip. Recovery accounting: - The ReAct and simple retry paths now report their uploads and commits. They previously committed silently, so any rescued volume never reached the metrics. An 8 GiB run that lost one 512-file batch to a rejected commit finished with all 40000 files on the Hub but 91 MB missing from done_bytes. Verified against the Hub with 40000 files / 8.00 GiB (37543 distinct contents, largest duplicate group 1272): - predicted saving 51,369,983 bytes; the run transferred 8,538,564,609 of 8,589,934,592 committed bytes -- short by exactly 51,369,983. - 12 of the 385 files that skipped their own PUT were downloaded and hashed: all matched, so no pointer was left dangling. - listing check: 40000 remote blobs, 0 missing, 0 extra, 0 size mismatches. - a fresh-state re-upload moved 64.81 MiB instead of 8 GiB and reported done_bytes exactly, confirming the recovery accounting fix.
path_in_repo="." (e.g. `ms upload REPO DIR .`) became a literal "./" prefix on every file, which the Hub rejects with E3021 "invalid commit action" on every commit, aborting the upload after 3 consecutive failed batches. Collapse ".", "./", "" and "/" to the repo root, normalize separators and ".", ".." segments, and refuse paths that escape the root. Applied in both upload_folder and upload_file; added regression tests. Verified end-to-end with a real 14131-file / 2.4GB dataset upload using path_in_repo=".": all files committed, 0 E3021, 0 aborts, server paths carry no "./" prefix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix
path_in_repo="."no longer fail. A.destination (e.g.ms upload REPO DIR .) was written into every commit action as a literal./path prefix, which the Hub rejected withE3021 "invalid commit action"on every commit — aborting the run after 3 consecutive failed batches..now correctly resolves to the repository root.Enhance
path_in_repois now normalized consistently acrossupload_folderandupload_file:.,./,/, and empty all map to the repo root; redundant separators and./..segments are collapsed; and paths that escape the repository root are rejected upfront.